Skip to content

Fix handling of dontcall attributes for arches that lower calls via fastSelectInstruction #153302

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 12, 2025

Conversation

dpaoliello
Copy link
Contributor

Recently my change to avoid duplicate dontcall attribute errors (#152810) caused the Clang Frontend/backend-attribute-error-warning.c test to fail on Arm32: https://lab.llvm.org/buildbot/#/builders/154/builds/20134

The root cause is that, if the default IFastSel path bails, then targets are given the opportunity to lower instructions via fastSelectInstruction. That's the path taken by Arm32 and since its implementation of selectCall didn't call diagnoseDontCall no error was emitted.

I've checked the other implementations of fastSelectInstruction and the only other one that lowers call instructions in WebAssembly, so I've fixed that too.

@llvmbot
Copy link
Member

llvmbot commented Aug 12, 2025

@llvm/pr-subscribers-backend-arm

@llvm/pr-subscribers-backend-webassembly

Author: Daniel Paoliello (dpaoliello)

Changes

Recently my change to avoid duplicate dontcall attribute errors (#152810) caused the Clang Frontend/backend-attribute-error-warning.c test to fail on Arm32: <https://lab.llvm.org/buildbot/#/builders/154/builds/20134>

The root cause is that, if the default IFastSel path bails, then targets are given the opportunity to lower instructions via fastSelectInstruction. That's the path taken by Arm32 and since its implementation of selectCall didn't call diagnoseDontCall no error was emitted.

I've checked the other implementations of fastSelectInstruction and the only other one that lowers call instructions in WebAssembly, so I've fixed that too.


Full diff: https://github.com/llvm/llvm-project/pull/153302.diff

2 Files Affected:

  • (modified) llvm/lib/Target/ARM/ARMFastISel.cpp (+1)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp (+2)
diff --git a/llvm/lib/Target/ARM/ARMFastISel.cpp b/llvm/lib/Target/ARM/ARMFastISel.cpp
index 54aa355d4db0a..14e1160e70dae 100644
--- a/llvm/lib/Target/ARM/ARMFastISel.cpp
+++ b/llvm/lib/Target/ARM/ARMFastISel.cpp
@@ -2504,6 +2504,7 @@ bool ARMFastISel::SelectCall(const Instruction *I,
   // Set all unused physreg defs as dead.
   static_cast<MachineInstr *>(MIB)->setPhysRegsDeadExcept(UsedRegs, TRI);
 
+  diagnoseDontCall(*CI);
   return true;
 }
 
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
index ec95e86e4fe3d..2666342d0c7b9 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
@@ -912,6 +912,8 @@ bool WebAssemblyFastISel::selectCall(const Instruction *I) {
 
   if (!IsVoid)
     updateValueMap(Call, ResultReg);
+
+  diagnoseDontCall(*Call);
   return true;
 }
 

@dpaoliello dpaoliello merged commit 2a82e23 into llvm:main Aug 12, 2025
12 checks passed
@dpaoliello dpaoliello deleted the dontcallfix branch August 12, 2025 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants